Skip to content

Conversation

xbinaryx
Copy link
Contributor

@xbinaryx xbinaryx commented Aug 11, 2025

Prerequisites checklist

What is the purpose of this pull request?

This PR prevents the lexer from receiving unresolved var() expressions by recursively resolving chained CSS custom properties. Previously the rule only performed a single-level replacement (e.g. --a: var(--b) → var(--b)), so chained variables like --a: var(--b); --b: 80px; could still leave var() in the value passed to the lexer, causing Error: Matching for a tree with var() is not supported.

What changes did you make? (Give an overview)

  • Implemented recursive/iterative resolution of custom properties so variables that reference other variables are resolved to their final, non-var() value before validation.
  • Added tests

Related Issues

Fixes #200

Is there anything you'd like reviewers to focus on?

@eslint-github-bot eslint-github-bot bot added the bug Something isn't working label Aug 11, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Aug 11, 2025
@lumirlumir lumirlumir added the accepted There is consensus among the team that this change meets the criteria for inclusion label Aug 12, 2025
@nzakas nzakas requested a review from Copilot August 14, 2025 15:50
@nzakas nzakas moved this from Needs Triage to Implementing in Triage Aug 14, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug in the no-invalid-properties ESLint rule where CSS custom properties containing chained var() references were not being fully resolved before validation, causing lexer errors. The fix implements recursive resolution of custom properties to ensure all variables are resolved to their final values before being passed to the CSS lexer.

Key changes:

  • Added recursive resolution logic for chained CSS custom properties
  • Refactored variable resolution to handle fallback chains and cycles
  • Added comprehensive test coverage for various chaining scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/rules/no-invalid-properties.js Implemented recursive variable resolution with new helper functions and refactored the main validation logic
tests/rules/no-invalid-properties.test.js Added extensive test cases covering valid and invalid chained variable scenarios

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this. Overall the code looks good, I think we just need some comments to help others understand how this works.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@nzakas nzakas merged commit f2ee3fd into eslint:main Aug 26, 2025
22 checks passed
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion bug Something isn't working contributor pool

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Bug: css variable not resolved in rule "css/no-invalid-properties"

3 participants